Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not assume working_directory is github.workspace #4103

Merged
merged 1 commit into from
Apr 25, 2024
Merged

Do not assume working_directory is github.workspace #4103

merged 1 commit into from
Apr 25, 2024

Conversation

ajfabbri
Copy link
Contributor

@ajfabbri ajfabbri commented Apr 9, 2024 •
edited
Loading

Setting working_directory for ansible-lint action would fail due to hard-coded .git, introduced in commit 6f728e0, when fetching .config/requirements-lock.txt.

This fix replaces .git with ${{ github.workspace }}/.git to make working_directory argument work again.

Tested that this does fix my github action, i.e.:

name: ansible-lint
on:
  pull_request:
    branches: ["main", "release/v*"]
jobs:
  build:
    name: Ansible Lint
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - name: Run ansible-lint
        uses: ajfabbri/ansible-lint@main
        # this is broken :-( https://github.com/ansible/ansible-lint/issues/3938
        with:
          working_directory: iac

Fixes #3938

@ajfabbri
Fix ansible#3938: don't assume working_directory is github.workspace
d58ce1c 
Setting `working_directory` for ansible-lint action would fail due to
hard-coded `.git`, introduced in commit 6f728e0, when fetching
`.config/requirements-lock.txt`.

This fix replaces `.git` with `${{ github.workspace }}/.git` to make
`working_directory` argument work again.
@ajfabbri ajfabbri marked this pull request as ready for review April 9, 2024 19:37
@ajfabbri ajfabbri requested a review from a team as a code owner April 9, 2024 19:37
@ajfabbri ajfabbri requested review from ssbarnea and Qalthos and removed request for a team April 9, 2024 19:37
@audgirka audgirka added the bug label Apr 10, 2024
@Qalthos Qalthos self-assigned this Apr 24, 2024
@Qalthos Qalthos merged commit 9a8db9f into ansible:main Apr 25, 2024
22 of 24 checks passed
@ssbarnea ssbarnea changed the title Fix #3938: don't assume working_directory is github.workspace Do not assume working_directory is github.workspace Apr 30, 2024
@commarla
Copy link

Thanks for this PR. I was waiting for this but then I encountered this bug actions/runner#2058
This solution does not working if running the action inside a container

@ptoscano
Copy link
Contributor

ptoscano commented Jun 5, 2024

Hi @ajfabbri,

this change unfortunately breaks the use case of checking out a repository in a subdirectory of the workspace and running ansible-lint in it, effectively reverting parts of #3905; in particular, now .git/ansible-lint-requirements.txt is assumed to be directly in the workspace, rather than where working_directory points to.

Example of workflow: https://github.com/RedHatInsights/ansible-collections-insights/blob/25a0213a4a955b44924c8439c3800f3f9157b76b/.github/workflows/ansible-lint.yml

      - name: Check out code
        uses: actions/checkout@v4
        with:
          path: ansible_collections/redhat/insights
[...]
      - name: Run ansible-lint
        uses: ansible/ansible-lint@v24
        with:
          working_directory: ansible_collections/redhat/insights

Failure due this change: https://github.com/RedHatInsights/ansible-collections-insights/actions/runs/9351931654/job/25740530216

Personally, I think working_directory ought to mean as base directory for everything, both git and sources; in case you want to run on a subdirectory of the git checkout, I'd recommend adding a separate parameter, e.g. repository_subdirectory (or something like that), and thus

      - uses: actions/checkout@v4
      - name: Run ansible-lint
        uses: ajfabbri/ansible-lint@main
        with:
          repository_subdirectory: iac

What do you think?

ptoscano added a commit to RedHatInsights/ansible-collections-insights that referenced this pull request Jun 5, 2024
@ptoscano
ci: re-add workaround for ansible-lint action regression
34b1323 
Due to recent changes in the ansible-lint action [1], the location of
the .git directory is hardcoded (ignoring "working_directory").
Hence, for now bring back the .git symlink workaround.

[1] ansible/ansible-lint#4103

Signed-off-by: Pino Toscano <[email protected]>
@ajfabbri
Copy link
Contributor Author

ajfabbri commented Jun 5, 2024

Thanks for the report. I think we should try to support your case (checkouts not in top-level workspace dir) as well as monorepo users (those that don't put ansible files at the top level of their repository). I'm not an expert here, but It seems like the source of the problem is the use of .git/ as a /tmp directory to work around issues with python requirements.txt hashing?

Two ideas to fix that

  1. Search ancestor dirs for .git until we find one (common git semantics for discovering repository from subdir). I.e. before the two steps that use the path ./git in this commit, we find the actual path to the first ancestor directory that contains .git/
  2. Try workarounds mentioned in actions/toolkit/issues/1035

DuckBoss pushed a commit to RedHatInsights/ansible-collections-insights that referenced this pull request Jun 5, 2024
@ptoscano @DuckBoss
ci: re-add workaround for ansible-lint action regression
1e3166d 
Due to recent changes in the ansible-lint action [1], the location of
the .git directory is hardcoded (ignoring "working_directory").
Hence, for now bring back the .git symlink workaround.

[1] ansible/ansible-lint#4103

Signed-off-by: Pino Toscano <[email protected]>
@tigattack tigattack mentioned this pull request Jul 22, 2024
Closed
This was referenced Sep 10, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Qalthos Qalthos Qalthos approved these changes

@ssbarnea ssbarnea Awaiting requested review from ssbarnea ssbarnea is a code owner automatically assigned from ansible/devtools

Assignees

@Qalthos Qalthos

Labels
bug
Projects
🧰 devtools project board
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

github action fails to download ansible-lint-requirements.txt, assumes .git dir exists in working directory
5 participants
@ajfabbri @commarla @ptoscano @Qalthos @audgirka